Conversation
|
Stack from ghstack (oldest at bottom): |
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/7992
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 3e124c0 with merge base 6fcce78 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
@pytorchbot label "topic: not user facing" |
iseeyuan
left a comment
iseeyuan
left a comment
There was a problem hiding this comment.
LGTM. Thanks @swolchok for this quick fix! @mcr229 and @digantdesai could you take a look?
|
@swolchok you mentioned rsqrt was used since Llama2. I'm wondering how it would help the overall performance. Probably it's fine because it's not a performance bottleneck there. It's used in RMSNorm, where the sqrt is applied to a scalar of RMS(x), not on each element of x. |
|
|
||
| def _test_rsqrt(self, inputs): | ||
| ( | ||
| Tester(self.Rsqrt(), inputs) |
There was a problem hiding this comment.
Nit: Do you want to improve this test by adding dynamic_shape support like this?
There was a problem hiding this comment.
improve this test by adding dynamic_shape support
it's a pointwise operator, why does it care about the shape? Everything is the same as sqrt including the test
There was a problem hiding this comment.
Yeah, just checking boxes, i.e. adding dynamic shape test for every operator.
There was a problem hiding this comment.
out of scope for this change! :)
I think I updated everywhere that needs updating? Test Plan: python -m unittest backends/xnnpack/test/ops/test_rsqrt.py
I think I updated everywhere that needs updating? Test Plan: python -m unittest backends/xnnpack/test/ops/test_rsqrt.py
4 participants
I think I updated everywhere that needs updating?
Test Plan: python -m unittest backends/xnnpack/test/ops/test_rsqrt.py